fix(templates): resolve GitHub API host from env in ralph-triage.js#1438
fix(templates): resolve GitHub API host from env in ralph-triage.js#1438omercangumus wants to merge 1 commit into
Conversation
🟠 Impact Analysis — PR #1438Risk tier: 🟠 HIGH 📊 Summary
🎯 Risk Factors
📦 Modules Affecteddocs (1 file)
root (2 files)
squad-cli (1 file)
squad-sdk (1 file)
templates (1 file)
tests (1 file)
This report is generated automatically for every PR. See #733 for details. |
🛫 PR Readiness Check
PR Scope: 🔧 Infrastructure
|
| Status | Check | Details |
|---|---|---|
| ✅ | Single commit | 1 commit — clean history |
| ✅ | Not in draft | Ready for review |
| ✅ | Branch up to date | Up to date with dev |
| ❌ | Copilot review | No Copilot review yet — it may still be processing |
| ✅ | Changeset present | Changeset file found |
| ✅ | Scope clean | No .squad/ or docs/proposals/ files |
| ✅ | No merge conflicts | No merge conflicts |
| ✅ | Copilot threads resolved | No Copilot review threads |
| ❌ | CI passing | 5 check(s) still running |
Files Changed (7 files, +164 −25)
| File | +/− |
|---|---|
.changeset/fix-1142-ralph-triage-ghe-api-host.md |
+10 −0 |
.squad-templates/ralph-triage.js |
+28 −6 |
docs/src/content/docs/scenarios/ci-cd-integration.md |
+2 −0 |
packages/squad-cli/templates/ralph-triage.js |
+28 −6 |
packages/squad-sdk/templates/ralph-triage.js |
+28 −6 |
templates/ralph-triage.js |
+28 −6 |
test/ralph-triage.test.ts |
+40 −1 |
Total: +164 −25
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
|
Thanks @omercangumus for this. It was indeed needed. Can you pleae also update the docs so users could be able to discover about this option? |
ralph-triage.js hardcoded hostname: 'api.github.com' in its https.request() call, so Squad Heartbeat (Ralph) failed with a 401 on GitHub Enterprise — the GITHUB_TOKEN there is only valid against the enterprise API host, not github.com. Added resolveGithubApiBase(), which picks the API base in order: GITHUB_API_URL (set by Actions on both github.com and GHE runners), then GITHUB_SERVER_URL + /api/v3, then https://api.github.com as a last-resort fallback. The request is now built from a URL object instead of a hardcoded hostname/path pair. No behavior change on github.com-hosted repos. Fixed in the canonical .squad-templates/ralph-triage.js and synced to the 3 mirror targets. Added unit tests covering all three resolution branches. Documented the env var resolution order in the CI/CD integration docs so GHE users can discover it. Closes bradygaster#1142 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ae02c82 to
58ca57b
Compare
|
good call, added a note in docs/src/content/docs/scenarios/ci-cd-integration.md right next to the cron callout, explains the GITHUB_API_URL/GITHUB_SERVER_URL fallback order and how to set it manually if running heartbeat outside Actions. lmk if you'd rather it live somewhere else |
tamirdresher
left a comment
There was a problem hiding this comment.
✅ Approved — security review passed.
Security checks:
- No SSRF risk: env vars (GITHUB_API_URL, GITHUB_SERVER_URL) are Actions-runner-controlled, not user-input-derived
- URL validation via
ew URL()\ prevents malformed URL injection - Trailing-slash stripping prevents path confusion
- Minimal export surface (only resolveGithubApiBase)
- No prompt injection vectors — pure HTTP infra
Correctness: Resolution order (GITHUB_API_URL → SERVER_URL+/api/v3 → fallback) matches GitHub's own documented env vars. Tests cover all 4 branches including trailing-slash normalization.
equire.main === module\ guard + exports for testability is textbook.
Good contribution, Ömer 👍
What
ralph-triage.jsnow resolves the GitHub REST API host from the environment instead of hardcodingapi.github.com.Why
Squad Heartbeat (Ralph)fails with a 401 on GitHub Enterprise.githubRequestJson()in.squad-templates/ralph-triage.jssent every request tohostname: 'api.github.com', but on GHE theGITHUB_TOKENis only valid against the enterprise API host (https://<ghe-host>/api/v3), so calls to github.com are rejected withBad credentials.Closes #1142
How
Added a
resolveGithubApiBase()helper that picks the API base in order:GITHUB_API_URL— set automatically by Actions on both github.com and GHE runners.GITHUB_SERVER_URL+/api/v3— fallback for GHE ifGITHUB_API_URLisn't set.https://api.github.com— fallback for local/non-Actions runs.githubRequestJson()now builds the request from aURLobject (${resolveGithubApiBase()}${pathname}) instead of a hardcodedhostname/pathpair, sohttps.request()picks up the correct protocol, host, and base path automatically. No behavior change on github.com-hosted repos, sinceGITHUB_API_URLthere already resolves tohttps://api.github.com.Fixed in the canonical
.squad-templates/ralph-triage.jsand propagated vianode scripts/sync-templates.mjs --syncto the 3 mirror targets (templates/,packages/squad-cli/templates/,packages/squad-sdk/templates/), matching the parity checks intest/template-sync.test.ts.Also added a
require.main === moduleguard around the script'smain()call and exportedresolveGithubApiBaseviamodule.exports, purely so the new unit tests can import and exercise the helper without invokingmain()(which needsGITHUB_TOKENand network access). No change to the script's behavior when run directly vianode ralph-triage.js.Testing
test/ralph-triage.test.tscovering all three resolution branches plus trailing-slash normalization. Verified they fail withresolveGithubApiBase is not a functionagainst the pre-fix code and pass against the fix.npx vitest run test/ralph-triage.test.ts test/template-sync.test.ts— 284 passed.npm run build— passes.npm run lint(tsc --noEmit) — passes.npm run lint:eslint— 0 errors (pre-existing warnings only, unrelated to this change).npm test(full suite) — no regressions from this change. A number of unrelated tests fail on this Windows dev machine (EBUSY temp-dir races, timing-sensitive timeouts, a locale-dependent number-formatting assertion) — confirmed identical failures on a cleanupstream/devcheckout before my changes, so they're pre-existing local-environment flakiness, not something this PR introduces.npx changeset add-equivalent manual file:.changeset/fix-1142-ralph-triage-ghe-api-host.md(patch bump for@bradygaster/squad-cliand@bradygaster/squad-sdk, since this touches governed template paths under both packages'templates/)PR Readiness Checklist
Branch & Commit
devdevgit diff --cached --stat)Build & Test
npm run buildpassesnpm testpasses (all tests relevant to this change; see testing notes above for pre-existing unrelated local failures)npm run lintpasses (type check clean)npm run lint:eslintpasses (0 errors)Changeset
Docs
Exports
Breaking Changes
None.
Waivers
None.